fix: Anthropic 与 Chat Completions 转换的思考块问题#4591
Open
clansty wants to merge 3 commits intoQuantumNous:mainfrom
Open
fix: Anthropic 与 Chat Completions 转换的思考块问题#4591clansty wants to merge 3 commits intoQuantumNous:mainfrom
clansty wants to merge 3 commits intoQuantumNous:mainfrom
Conversation
Contributor
WalkthroughAdds nullable opaque reasoning/signature fields to Claude and OpenAI DTOs, updates Claude↔OpenAI conversion to propagate reasoning content and opaque signatures (with defensive nil handling and send-tracking), and adds tests covering request/stream/response conversions and edge cases. ChangesOpaque reasoning/signature propagation
Sequence Diagram(s)sequenceDiagram
participant Claude
participant Converter
participant OpenAI
participant RelayInfo
Claude->>Converter: emits message (may include thinking + signature)
Converter->>RelayInfo: read/update ClaudeConvertInfo (ReasoningContent/ReasoningOpaque)
Converter->>OpenAI: sends OpenAI request with ReasoningContent & ReasoningOpaque
OpenAI->>Converter: streams deltas (content, reasoning content, reasoning opaque)
Converter->>RelayInfo: accumulate reasoning content/opaque, set SentThinking/SentSignature
Converter->>Claude: emit thinking content_block, then signature_delta, then tool_use
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@service/convert_test.go`:
- Around line 88-104: The test
TestClaudeToOpenAIRequestSkipsEmptyThinkingOnlyMessage currently only covers a
nil Thinking pointer; update tests to cover the non-nil empty-string pointer
case as well (create a dto.ClaudeMediaMessage with Thinking set to
common.GetPointer("")) to exercise ClaudeToOpenAIRequest through
ParseContent/ToolCalls, or rename the test to indicate it only covers the
nil-Thinking scenario; ensure the added sub-case asserts that
openAIRequest.Messages remains empty and that no tool call/ReasoningContent is
produced.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: e2db6b79-ff64-4e46-95a9-1d92e2f2905c
📒 Files selected for processing (4)
dto/openai_request.godto/openai_response.goservice/convert.goservice/convert_test.go
Comment on lines
+88
to
+104
| func TestClaudeToOpenAIRequestSkipsEmptyThinkingOnlyMessage(t *testing.T) { | ||
| claudeRequest := dto.ClaudeRequest{ | ||
| Model: "deepseek-v4-pro", | ||
| Messages: []dto.ClaudeMessage{ | ||
| { | ||
| Role: "assistant", | ||
| Content: []dto.ClaudeMediaMessage{ | ||
| {Type: "thinking"}, | ||
| }, | ||
| }, | ||
| }, | ||
| } | ||
|
|
||
| openAIRequest, err := ClaudeToOpenAIRequest(claudeRequest, testRelayInfo()) | ||
| require.NoError(t, err) | ||
| require.Empty(t, openAIRequest.Messages) | ||
| } |
Contributor
There was a problem hiding this comment.
Test name vs test fixture mismatch — {Type: "thinking"} has a nil Thinking pointer, not an empty string.
TestClaudeToOpenAIRequestSkipsEmptyThinkingOnlyMessage tests the nil-Thinking case only. A thinking block with an explicit Thinking: common.GetPointer("") (non-nil pointer to empty string) would set ReasoningContent to &"" but the message would still be dropped by the ParseContent()/ToolCalls gate — that path is untested. The current test still validates the primary use case, but the name implies coverage it doesn't provide.
🛡️ Proposed additional sub-case
func TestClaudeToOpenAIRequestSkipsEmptyThinkingOnlyMessage(t *testing.T) {
+ // Case 1: nil Thinking pointer
claudeRequest := dto.ClaudeRequest{
Model: "deepseek-v4-pro",
Messages: []dto.ClaudeMessage{
{
Role: "assistant",
Content: []dto.ClaudeMediaMessage{
{Type: "thinking"},
},
},
},
}
openAIRequest, err := ClaudeToOpenAIRequest(claudeRequest, testRelayInfo())
require.NoError(t, err)
require.Empty(t, openAIRequest.Messages)
+
+ // Case 2: non-nil pointer to empty string
+ emptyStr := ""
+ claudeRequest2 := dto.ClaudeRequest{
+ Model: "deepseek-v4-pro",
+ Messages: []dto.ClaudeMessage{
+ {
+ Role: "assistant",
+ Content: []dto.ClaudeMediaMessage{
+ {Type: "thinking", Thinking: &emptyStr},
+ },
+ },
+ },
+ }
+ openAIRequest2, err2 := ClaudeToOpenAIRequest(claudeRequest2, testRelayInfo())
+ require.NoError(t, err2)
+ require.Empty(t, openAIRequest2.Messages)
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@service/convert_test.go` around lines 88 - 104, The test
TestClaudeToOpenAIRequestSkipsEmptyThinkingOnlyMessage currently only covers a
nil Thinking pointer; update tests to cover the non-nil empty-string pointer
case as well (create a dto.ClaudeMediaMessage with Thinking set to
common.GetPointer("")) to exercise ClaudeToOpenAIRequest through
ParseContent/ToolCalls, or rename the test to indicate it only covers the
nil-Thinking scenario; ensure the added sub-case asserts that
openAIRequest.Messages remains empty and that no tool call/ReasoningContent is
produced.
Contributor
Author
|
已测试能解决问题 |
clansty
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
对于入口是 Anthropic，出口是 OpenAI Chat Completions 兼容 API 的情况，使用 OpenCode 调用 deepseek v4 pro 或者 kimi k2.6 之类的模型，会出现在第一个工具调用之后提示
的情况，有以下两个原因：
对于第一个问题，我已经实现了思考块的转换。对于第二个问题，我认为签名字段和思考文本内容字段一样，就算是空字符串也不能是 null 值
有关思考块转换，我现在的实现是，在 Chat Completions 接口上，使用
reasoning_opaque字段存储签名值。Chat Responses 并没有一个标准的思考签名存储字段，但是据我的观察，其他的应用程序有在使用reasoning_opaque传输签名。但是对于来回都是同一渠道并且国产模型的场合，似乎也不会设计到思考签名。所以这大概算是一个保守设计Summary
reasoning_opaqueas an OpenAI-compatible extension field for opaque reasoning metadata.thinkingtoreasoning_contentandsignaturetoreasoning_opaque.reasoning_content/reasoning_opaqueback to Anthropicthinkingblocks before tool-use replay.Why
DeepSeek thinking mode requires assistant reasoning content to be replayed when a tool call occurs. Without preserving this metadata through Anthropic/OpenAI-compatible conversion, follow-up tool-result requests can fail with a missing reasoning-content error. Anthropic also carries signed thinking via
signature, so this keeps the opaque signature separate from visible reasoning text.Tests
go test ./servicego test ./relay/channel/deepseekNotes
reasoning_opaqueis treated as a compatibility extension for new-api/OpenAI-compatible traffic, not as visible reasoning text.go test ./relay/channel/claudecurrently has unrelated file-content conversion test failures on this branch.Summary by CodeRabbit
New Features
Bug Fixes
Tests